Conversation
8ddfd65 to
786f289
Compare
04a1ef9 to
a54e804
Compare
ef9d6ce to
4b74f89
Compare
There was a problem hiding this comment.
Nice work! It looks really good :)
I'm a bit confused by the commit message to the Add support for pg_regress tests in Meson commit. There is no install added anywhere as far as I can see?
I'm not sure the last commit belong in this PR at all, but yeah. Contributing.md definitely needs updating or removing.
Spelling:
First commit message:
expereince->experience * 2
Second commit message:
amke -> make
Sixth commit message:
There is an extra "this" here: "Also this apparently this fixed"
Ninth commit message:
Meosn -> Meson
May I suggest echo set spell >> .vimrc? 😜
| '-fexcess-precision=standard', | ||
| ]), language: ['c']) | ||
|
|
||
| # TODO: Why does it seems like they sometimes are in pkglibdir and other times in libdir? |
There was a problem hiding this comment.
It's because there are some shenanigans going on with cross compilation I think.
There was a problem hiding this comment.
Possibly but I do not understand it. Would it be ok to merge this without understanding or should I investigate this before we merge the PR?
AndersAstrand
left a comment
There was a problem hiding this comment.
Could the Makefile just invoke meson after this if we need make for a little longer?
To give a more modern development experience we plan to replace our current make and PGXS based build system with one which uses Meson and pg_config to achieve the same thing. Potential advantages: - More modern and pleasant experience - Parallel tests - Allows for Windows support Potential disadvantages: - We may have to manually implement support for odd platforms
This is split out from the previous commit to make the history easier to read and the commits easier to review.
One of the weird things with the Meson tests for pg_tde is that they are actually install tests so we need to manually run meson install before running meson test.
One of the nice advantages of meson is that test can be run in parallel which we now can take advantage of.
Based on Makefile.darwin in the PostgreSQL project.
Since we plan to only support Meson builds in the future let's move all of our CI jobs over to using Meson. For simplicty we put the build directory outside of the git repository. Also this apparently fixed a bug in our code formatting job for the frotnend tools so let's also commit the changed formatting. Disable the test timeout in meson since the default timeout is an issue when running with sanitizers.
While we are moving our development builds and CI directly from make to Meson our QA farm and packaging needs more time to migrate to Meson, so to make sure our make file does not break we add a simple CI job which builds pg_tde using make and then runs the test cases.
|
Given that the reason that we are keeping make is because the build engineers are very busy right now I do not think it is an option to try to rewrite the makefile to wrap meson. Things could break then. Maybe as a next step when things have calmed down for them. But in that case we may not even need to keep it around at all. |
We'll also need to inform them of the move of |
The instructions are still wrong after this but I did some quick corrections to even be able to update it for adding Meson support.
Not due to this PR. That was intended for a separate PR. And accidentally left in by me. Lifted it out now. |
Meson gives a more modern development experience while making some features easier to implement in the future. This Meson file calls
pg_configdirectly to get the paths but does not get any of the compiler flags frompg_config.This PR moves the CI to use Meson but since the build and QA teams need extra time to migrate to Meson we keep the makefile and add a new CI job which checks so we do not break it.
Please tell me if this PR is too big and should be split for ease of review.
Pros:
pg_oidc_validatorCons:
Makefile.{plaform}filespg_tdebut for our other extensions that is an issue